Dynamic storage support #982
Conversation
Dentosal
requested review from
MitchTurner,
Voxelot and
xgreenx
as code owners
PR SummaryIntroduces variable-sized contract storage and updates the VM, assembler, and consensus to support it.
Written by Cursor Bugbot for commit 47d787d. This will update automatically on new commits. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| self, | ||
| interpreter: &mut Interpreter<M, S, Tx, Ecal, V>, | ||
| ) -> IoResult<ExecuteState, S::DataError> { | ||
| let (a, b, c, d) = self.unpack(); |
There was a problem hiding this comment.
It would be nice if we used readable names here like in hte opcode definition
fuel-vm/src/interpreter/storage.rs
Outdated
| .storage | ||
| .contract_state(&contract_id, &key) | ||
| .map_err(RuntimeError::Storage)? | ||
| .map(|v| v.as_ref().as_ref().to_vec()) |
There was a problem hiding this comment.
You can convert the value into owned type (because, under the hood, it is already owned). Overwise you create vector two times(one when we create value, and another here).
| let offset = if offset == u64::MAX { | ||
| value.len() | ||
| } else { |
There was a problem hiding this comment.
Did we specif this behaviour in the specification?
There was a problem hiding this comment.
Yep.
Passing in
u64::MAXin$rCwill cause the write to happen at the end of the slot, without needing to read the slot length first.
| let dst = self.memory.as_mut().write(owner, dst_ptr, len)?; | ||
| let value = self | ||
| .storage | ||
| .contract_state(&contract_id, &key) |
There was a problem hiding this comment.
Maybe it is better to use read_bytes here? In this case we will avoud creation of the vector on the fuel-core side
There was a problem hiding this comment.
Not sure which read_bytes you're talking about?
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| interpreter: &mut Interpreter<M, S, Tx, Ecal, V>, | ||
| ) -> IoResult<ExecuteState, S::DataError> { | ||
| match self { | ||
| match dbg!(self) { |
There was a problem hiding this comment.
Debug macro left in production instruction execution code
High Severity
A dbg!(self) macro was accidentally left in the main Execute::execute implementation for Instruction. This will print every single VM instruction to stderr during execution, causing significant performance degradation and producing unwanted debug output in production. The match dbg!(self) should be match self.
2 participants
Implements dynamic storage instructions as described in FuelLabs/fuel-specs#640
The storage initialization in
Createtransactions is left as-is, i.e. it only allows 32 byte slots. This feature isn't used much, and I don't think it's worth it to continue supporting.Checklist
Before requesting review